src/main/java/com/p2ps/config/RoomSubscriptionInterceptor.java (1)

47-49: Anonymous authentication is not currently enabled in this application; this defensive check is optional.

Anonymous auth is explicitly disabled in SecurityConfig (no .anonymous() call), and StompJwtAuthInterceptor returns null when no JWT is present, so the principal remains null. The existing instanceof check on line 47 correctly rejects this case. However, adding an explicit AnonymousAuthenticationToken check is a reasonable defense-in-depth improvement to guard against future configuration changes that might enable anonymous auth without realizing the WebSocket impact.

Optional defensive improvement

 import org.springframework.security.core.Authentication;
+import org.springframework.security.authentication.AnonymousAuthenticationToken;
@@
-                if (!(principal instanceof Authentication authentication) || !authentication.isAuthenticated()) {
+                if (!(principal instanceof Authentication authentication)
+                        || !authentication.isAuthenticated()
+                        || authentication instanceof AnonymousAuthenticationToken) {
                     logger.warn("Security Alert: Blocked subscription attempt without authenticated principal");
                     return null;
                 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/com/p2ps/config/RoomSubscriptionInterceptor.java` around lines
47 - 49, The subscription guard in RoomSubscriptionInterceptor currently checks
that principal is an Authentication and authenticated but does not explicitly
reject AnonymousAuthenticationToken; add an additional defensive check to treat
AnonymousAuthenticationToken as unauthenticated by updating the conditional that
inspects principal (used where principal and Authentication are referenced) to
also return null if principal is an instance of
org.springframework.security.authentication.AnonymousAuthenticationToken,
ensuring the method (same block that previously returned null) continues to log
the warning and exit when anonymous auth appears in the future.

src/main/java/com/p2ps/config/StompJwtAuthInterceptor.java (2)

61-71: Consider simplifying header lookup with a loop.

The sequential null checks for different header names can be simplified.

♻️ Suggested simplification

     private String resolveToken(StompHeaderAccessor accessor) {
-        String headerToken = accessor.getFirstNativeHeader("Authorization");
-        if (headerToken == null) {
-            headerToken = accessor.getFirstNativeHeader("authorization");
-        }
-        if (headerToken == null) {
-            headerToken = accessor.getFirstNativeHeader("token");
-        }
-        if (headerToken == null) {
-            headerToken = accessor.getFirstNativeHeader("access_token");
-        }
+        String headerToken = Stream.of("Authorization", "authorization", "token", "access_token")
+                .map(accessor::getFirstNativeHeader)
+                .filter(Objects::nonNull)
+                .findFirst()
+                .orElse(null);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/com/p2ps/config/StompJwtAuthInterceptor.java` around lines 61 -
71, In resolveToken(StompHeaderAccessor), replace the repeated null-checks for
different header names by iterating a list/array of candidate header keys (e.g.,
"Authorization", "authorization", "token", "access_token") and returning the
first non-null value; update the method to loop through these keys calling
accessor.getFirstNativeHeader(key) and return when a non-null token is found,
otherwise continue to null return — this keeps the lookup concise and preserves
current behavior while centralizing the header list for easy future changes.

---

86-97: Duplicate extractBearerToken implementation.

This method duplicates JwtAuthFilter.extractBearerToken(). Consider reusing the existing method to maintain consistency and reduce duplication.

♻️ Proposed fix

-    private String extractBearerToken(String authorizationHeader) {
-        if (authorizationHeader == null || authorizationHeader.isBlank()) {
-            return null;
-        }
-
-        String token = authorizationHeader.trim();
-        if (token.regionMatches(true, 0, "Bearer ", 0, 7)) {
-            return token.substring(7).trim();
-        }
-
-        return token;
-    }

Then update resolveToken to use jwtAuthFilter.extractBearerToken(headerToken):

         if (headerToken != null) {
-            return extractBearerToken(headerToken);
+            return jwtAuthFilter.extractBearerToken(headerToken);
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/com/p2ps/config/StompJwtAuthInterceptor.java` around lines 86 -
97, The extractBearerToken in StompJwtAuthInterceptor duplicates logic already
present in JwtAuthFilter.extractBearerToken; remove the duplicate method and
call the existing JwtAuthFilter.extractBearerToken(...) instead (inject or use
the existing jwtAuthFilter instance) from StompJwtAuthInterceptor.resolveToken
where headerToken is processed so token extraction is delegated to
JwtAuthFilter.extractBearerToken and duplication is eliminated.

src/main/java/com/p2ps/config/WebSocketConfig.java (1)

27-38: Javadoc is outdated.

The constructor Javadoc only documents subscriptionInterceptor but now accepts three interceptors. Update it to reflect all parameters.

📝 Suggested Javadoc update

     /**
-     * Constructor injection for the subscription security interceptor.
-     * `@param` subscriptionInterceptor the interceptor validating inbound traffic
+     * Constructor injection for WebSocket interceptors.
+     * `@param` jwtHandshakeInterceptor validates JWT tokens during WebSocket handshake
+     * `@param` stompJwtAuthInterceptor authenticates STOMP CONNECT frames using JWT
+     * `@param` subscriptionInterceptor validates room subscription requests
      */

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/com/p2ps/config/WebSocketConfig.java` around lines 27 - 38, The
constructor Javadoc for WebSocketConfig is outdated and only documents
subscriptionInterceptor; update the Javadoc block above the WebSocketConfig(...)
constructor to list and describe all three parameters (JwtHandshakeInterceptor
jwtHandshakeInterceptor, StompJwtAuthInterceptor stompJwtAuthInterceptor, and
RoomSubscriptionInterceptor subscriptionInterceptor), ensuring each `@param` tag
accurately describes the role of that interceptor in the constructor.

src/test/java/com/p2ps/auth/security/SecurityConfigTest.java (1)

50-67: CORS test has hardcoded expected values.

The test asserts specific CORS configuration values (e.g., http://localhost:5173). While this provides regression protection, consider adding a comment noting these values must stay in sync with SecurityConfig, or extract them as constants shared between production code and tests.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/java/com/p2ps/auth/security/SecurityConfigTest.java` around lines 50
- 67, The test corsConfigurationSource_ConfiguredCorrectly in SecurityConfigTest
hardcodes CORS values that must match SecurityConfig.corsConfigurationSource;
either add a concise comment in the test referencing
SecurityConfig.corsConfigurationSource to warn future editors that these
expected values must stay in sync, or better: extract the hardcoded values into
public constants on SecurityConfig (e.g., ALLOWED_ORIGINS, ALLOWED_METHODS,
ALLOWED_HEADERS, ALLOW_CREDENTIALS) and use those constants in the test when
asserting (via SecurityConfig.ALLOWED_ORIGINS, etc.) so the production
configuration and test expectations remain synchronized.

src/test/java/com/p2ps/auth/security/JwtAuthFilterTest.java (1)

218-231: Assert filter-chain continuation in this path as well.

Line 218-231 validates authentication state but does not assert that the request continues down the chain. Add verify(filterChain).doFilter(request, response) for parity with the other doFilterInternal tests.

Suggested assertion add-on

         jwtAuthFilter.doFilterInternal(request, response, filterChain);

         assertNotNull(SecurityContextHolder.getContext().getAuthentication());
         assertEquals("test@test.com", SecurityContextHolder.getContext().getAuthentication().getPrincipal());
+        verify(filterChain).doFilter(request, response);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/java/com/p2ps/auth/security/JwtAuthFilterTest.java` around lines 218
- 231, The test method bareTokenWithoutBearerPrefix in JwtAuthFilterTest asserts
authentication but doesn't verify the filter chain was continued; update this
test to also verify the chain by adding a verification that
filterChain.doFilter(request, response) was called (i.e., call
verify(filterChain).doFilter(request, response)) after the existing assertions
so this path mirrors the other doFilterInternal tests.

src/test/java/com/p2ps/config/StompJwtAuthInterceptorTest.java (1)

45-60: Avoid stubbing authenticateToken(null) in no-token cases; verify no auth call instead.

These tests currently pass even if the interceptor incorrectly invokes authentication with null. Prefer asserting verify(jwtAuthFilter, never()).authenticateToken(any()) in no-token paths.

Also applies to: 170-203

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/java/com/p2ps/config/StompJwtAuthInterceptorTest.java` around lines
45 - 60, Update the test preSend_ConnectWithoutTokenPassesThrough to assert that
JwtAuthFilter.authenticateToken is never called rather than stubbing
authenticateToken(null); specifically, remove the
when(jwtAuthFilter.authenticateToken(null)) stub and add verify(jwtAuthFilter,
never()).authenticateToken(any()) after calling StompJwtAuthInterceptor.preSend,
referencing JwtAuthFilter.authenticateToken and StompJwtAuthInterceptor.preSend
to locate the code; apply the same change pattern to the other tests noted
(lines ~170-203) that handle "no-token" scenarios so they verify no
authentication invocation instead of stubbing null.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes